[Xamarin.Android.Build.Tasks] perf improvements for LlvmIrGenerator by jonathanpeppers · Pull Request #7626 · dotnet/android

jonathanpeppers · 2022-12-09T16:27:08Z

Reviewing dotnet trace output for a dotnet new maui app:

dotnet trace collect --format speedscope -- .\bin\Release\dotnet\dotnet.exe build -bl --no-restore foo.csproj

21% of the time in <GenerateJavaStubs/> is spent in:

249.60ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.Andro...

Drilling down, some of the hot spots are:

68.64ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WriteString(class System.String,class System.St
40.39ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WritePointer(class Xamarin.Android.Tasks.LLVMIR

These seem to be places we create intermediate string or byte[] objects.

The types of things I solved so far:

I audited every $"" to see if we could call TextWriter.Write() or StringBuilder.Append() multiple times to avoid creating a string.
Use ArrayPool in combination with Encoding.GetBytes().
Use char overloads, where we were using a 1-length string.

Hopefully, I've changed no behavior at all here, and we've just saved a lot of string and byte[] allocations.

After these changes go in, we can probably profile and find other areas to improve.

Testing the same dotnet new maui app, the overall time is now:

190.97ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.AndroidT

I think this will save ~60ms on incremental builds of the .NET MAUI project template. Larger projects with more Java subclasses could save even more.

Reviewing `dotnet trace` output for a `dotnet new maui` app: dotnet trace collect --format speedscope -- .\bin\Release\dotnet\dotnet.exe build -bl --no-restore foo.csproj 21% of the time in `<GenerateJavaStubs/>` is spent in: 249.60ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.Andro... Drilling down, some of the hot spots are: 68.64ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WriteString(class System.String,class System.St 40.39ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WritePointer(class Xamarin.Android.Tasks.LLVMIR Which seem to be places The types of things I solved so far: * I audited every `$""` to see if we could call `TextWriter.Write()` or `StringBuilder.Append()` multiple times to avoid creating a string. * Use `ArrayPool` in combination with `Encoding.GetBytes()`. * Use `char` overloads, where we were using a 1-length `string`. Hopefully, I've changed no behavior at all here, and we've just saved a lot of `string` and `byte[]` allocations. After these changes go in, we can probably profile and find other areas to improve. Testing the same `dotnet new maui` app, the overall time is now: 190.97ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.AndroidT I think this will save ~60ms on incremental builds of the .NET MAUI project template. Larger projects with more Java subclasses could save even more.

grendello

The changes make code slightly less readable, but savings are large enough to justify it.

dellis1972

Looks good if its green :D

jonathanpeppers · 2022-12-09T17:08:23Z

The changes make code slightly less readable, but savings are large enough to justify it.

I like $"" better too in most of these places...

grendello · 2022-12-09T17:18:26Z

The changes make code slightly less readable, but savings are large enough to justify it.

I like $"" better too in most of these places...

The main reason I used it was to be able to see what syntax is generated by the line. I wonder, maybe a comment with target syntax above the relevant code would be a good idea?

jonathanpeppers · 2022-12-09T17:26:36Z

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs

-			output.WriteLine ($"[{bufferSize} x {irType}] zeroinitializer, align {GetAggregateAlignment ((int)smi.BaseTypeSize, size)}");
+			output.Write ('[');
+			output.Write (bufferSize);
+			output.Write (" x ");
+			output.Write (irType);
+			output.Write ("] zeroinitializer, align ");
+			output.WriteLine (GetAggregateAlignment ((int) smi.BaseTypeSize, size));


For this line, for example, I could add a comment like:

// $"[{bufferSize} x {irType}] zeroinitializer, align {GetAggregateAlignment ((int)smi.BaseTypeSize, size)}"

I'll go through and do that, but let's see if the PR has any failing tests or anything.

Adding some comments where `$""` was more readable before

Classic tests were failing with `unable to load assembly System.Buffers.dll` on Windows, because they used the `GeneratePackageManagerJava` MSBuild task directly from C# in a .NET framework test project. To fix this, I brought over an assembly-binding redirect we already had in xabuild's `App.config`.

* main: [Xamarin.Android.Build.Tasks] use %(TrimmerRootAssembly.RootMode) All (dotnet#7651) Bump to dotnet/installer@47a747f 8.0.100-alpha.1.22616.7 (dotnet#7647) Bump to dotnet/installer@167a4ed 8.0.100-alpha.1.22611.1 (dotnet#7630) Bump to xamarin/Java.Interop/main@f8d77fa (dotnet#7638) [ci] Fix Designer test paths (dotnet#7635) [Xamarin.Android.Build.Tasks] perf improvements for LlvmIrGenerator (dotnet#7626) [Xamarin.Android.Build.Tasks] AutoImport `*.webp` files (dotnet#7601) Localized file check-in by OneLocBuild Task (dotnet#7632) Bump $(ProductVersion) to 13.2.99 Bump to xamarin/monodroid@c0c933b7 (dotnet#7633) [Xamarin.Android.Build.Tasks] Fix aapt_rules.txt corruption (dotnet#7587) [Xamarin.Android.Build.Tasks] Add XA1031 error (dotnet#7448) Bump to xamarin/Java.Interop/main@149d70f (dotnet#7625) [CODEOWNERS] More updates to CODEOWNERS (dotnet#7628) [Xamarin.Android.Build.Tasks] avoid `File.Exists()` check (dotnet#7621)

…7626) Reviewing `dotnet trace` output for a `dotnet new maui` app: dotnet trace collect --format speedscope -- .\bin\Release\dotnet\dotnet.exe build -bl --no-restore foo.csproj 21% of the time in `<GenerateJavaStubs/>` is spent in: 249.60ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.Andro... Drilling down, some of the hot spots are: 68.64ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WriteString(class System.String,class System.St 40.39ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrGenerator.WritePointer(class Xamarin.Android.Tasks.LLVMIR Which seem to be places The types of things I solved so far: * I audited every `$""` to see if we could call `TextWriter.Write()` or `StringBuilder.Append()` multiple times to avoid creating a string. * Use `ArrayPool` in combination with `Encoding.GetBytes()`. * Use `char` overloads, where we were using a 1-length `string`. Hopefully, I've changed no behavior at all here, and we've just saved a lot of `string` and `byte[]` allocations. After these changes go in, we can probably profile and find other areas to improve. Testing the same `dotnet new maui` app, the overall time is now: 190.97ms xamarin.android.build.tasks!Xamarin.Android.Tasks.LLVMIR.LlvmIrComposer.Write(value class Xamarin.Android.Tools.AndroidT I think this will save ~60ms on incremental builds of the .NET MAUI project template. Larger projects with more Java subclasses could save even more. Other changes: * Added comments where `$""` was more readable before * Fix new usage of System.Buffers in tests Classic tests were failing with `unable to load assembly System.Buffers.dll` on Windows, because they used the `GeneratePackageManagerJava` MSBuild task directly from C# in a .NET framework test project. To fix this, I brought over an assembly-binding redirect we already had in xabuild's `App.config`.

jonathanpeppers requested a review from grendello December 9, 2022 16:27

jonathanpeppers requested a review from dellis1972 as a code owner December 9, 2022 16:27

grendello approved these changes Dec 9, 2022

View reviewed changes

dellis1972 approved these changes Dec 9, 2022

View reviewed changes

jonathanpeppers commented Dec 9, 2022

View reviewed changes

jonathanpeppers added 3 commits December 9, 2022 14:51

Add readability comments

5ccd42a

Adding some comments where `$""` was more readable before

Remove newline

3686b3b

jonathanpeppers merged commit 0f7a150 into dotnet:main Dec 13, 2022

jonathanpeppers deleted the llvmgenerator-performance branch December 13, 2022 14:24

github-actions bot locked and limited conversation to collaborators Jan 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Xamarin.Android.Build.Tasks] perf improvements for LlvmIrGenerator#7626

[Xamarin.Android.Build.Tasks] perf improvements for LlvmIrGenerator#7626
jonathanpeppers merged 4 commits intodotnet:mainfrom
jonathanpeppers:llvmgenerator-performance

jonathanpeppers commented Dec 9, 2022 •

edited

Loading

Uh oh!

grendello left a comment

Uh oh!

dellis1972 left a comment

Uh oh!

jonathanpeppers commented Dec 9, 2022

Uh oh!

grendello commented Dec 9, 2022

Uh oh!

jonathanpeppers Dec 9, 2022 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jonathanpeppers commented Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grendello left a comment

Choose a reason for hiding this comment

Uh oh!

dellis1972 left a comment

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers commented Dec 9, 2022

Uh oh!

grendello commented Dec 9, 2022

Uh oh!

jonathanpeppers Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jonathanpeppers commented Dec 9, 2022 •

edited

Loading

jonathanpeppers Dec 9, 2022 •

edited

Loading